Skip to content

Replace StubCorveilTaskBackend with real CorveilTaskBackend (CROW-495)#501

Merged
dgershman merged 3 commits into
mainfrom
feature/crow-495-corveil-task-backend
Jun 12, 2026
Merged

Replace StubCorveilTaskBackend with real CorveilTaskBackend (CROW-495)#501
dgershman merged 3 commits into
mainfrom
feature/crow-495-corveil-task-backend

Conversation

@dgershman

Copy link
Copy Markdown
Collaborator

Summary

  • Mirrors JiraTaskBackend over the corveil CLI: corveil task get/list/update/create for all six TaskBackend methods. Declares [batchedQuery, projectBoardStatus].
  • Swaps the .corveil factory case + fetchTicket delegation in ProviderManager; adds an additionalCorveilHosts URL-routing list parallel to GitLab.
  • Adds WorkspaceInfo.corveilHost (Codable), a "Corveil host" row + new Corveil task-backend tag in WorkspaceFormView, and a per-workspace CorveilConfig fan-out in IssueTracker.refresh.
  • Removes StubCorveilTaskBackend.swift and the associated stub test; updates ADR 0005 Decision + Migration tables and documents the .inReview → in_progress lossy mapping.
  • Adds CorveilTaskBackendTests.swift (23 tests against FakeShellRunner) + five Corveil routing tests in ProviderManagerTests.

Status mapping: Crow → Corveil is backlog/ready → open, inProgress/inReview → in_progress (the only lossy mapping — corveil has no review-distinct status), done → closed. Reverse: open → .ready, in_progress → .inProgress, closed → .done.

Closes #495.

Test plan

  • swift test --package-path Packages/CrowProvider — 86 tests, 0 failures (46 BackendsTests + 23 new CorveilTaskBackendTests + 17 JiraTaskBackendTests + 49 ProviderManagerTests).
  • swift test --package-path Packages/CrowCore — 271 tests, 0 failures (WorkspaceInfo Codable round-trip with new corveilHost field included).
  • swift test --package-path Packages/CrowUI — 36 tests, 0 failures.
  • swift test --filter IssueTracker at repo root — 120 tests, 0 failures (no regression from the new fetchCorveilIssues path).
  • rg StubCorveilTaskBackend Sources/ Packages/ docs/ returns zero matches.
  • Smoke test in-app: launch Crow, open Workspace settings, add a workspace with the Corveil task backend, save/reload to confirm corveilHost round-trips through the config file. Paste https://corveil.io/dashboard/tasks/42 into the ticket-attach flow — ProviderManager.detect should route to .corveil and fetchTask should run (failing only on corveil login, not the old unimplemented stub error).

🤖 Generated with Claude Code

Mirrors JiraTaskBackend over the `corveil` CLI surface so Corveil-tasked
sessions work end-to-end (paired with a GitHub/GitLab CodeBackend via
Session.codeProvider). Declares `[batchedQuery, projectBoardStatus]` —
batched on the assumption corveil#1364's bulk `--ids` has landed; the
project-board status surface is real, with `.inReview` mapping lossily
to corveil's `in_progress` (no review-distinct intermediate exists).

- New `CorveilTaskBackend.swift` with `CorveilConfig` + `CorveilTaskID`
  parser; all six TaskBackend methods build `corveil task …` argv via
  the shared `ShellRunner`. Auth-failure output rewrites to a clear
  "run `corveil login`" hint, matching the Jira pattern.
- `ProviderManager` factory swaps the `.corveil` arm to the real backend
  and `fetchTicket` delegates Corveil URLs the same way Jira does; URL
  detection takes an `additionalCorveilHosts` list parallel to GitLab.
- `WorkspaceInfo.corveilHost` (Codable, decoded with `decodeIfPresent`)
  plus a "Corveil host" row in `WorkspaceFormView` and a `Corveil` tag
  in the Task Backend picker.
- `IssueTracker` dedupes per-workspace `CorveilConfig`s and adds a
  `fetchCorveilIssues` path next to `fetchJiraIssues`.
- Removes `StubCorveilTaskBackend.swift` and the associated stub test;
  ADR 0005 swaps the Corveil row in the Decision and Migration tables
  and documents the `.inReview → in_progress` asymmetry.
- New `CorveilTaskBackendTests.swift` (23 tests against `FakeShellRunner`,
  covering argv shape, JSON parsing, status round-trips, and auth-error
  surfacing); `ProviderManagerTests` gains five Corveil routing tests.

🐦‍⬛ Generated with Claude Code, orchestrated by Crow

Co-Authored-By: Claude <[email protected]>
Crow-Session: F5DEB949-5B6F-4878-9D00-306312BBE797
@dgershman dgershman requested a review from dhilgaertner as a code owner June 11, 2026 23:53
@dgershman dgershman added the crow:merge Crow auto-merge on green label Jun 11, 2026

@dhilgaertner dhilgaertner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code & Security Review

Real CorveilTaskBackend cleanly mirrors JiraTaskBackend, the ADR 0005 updates are accurate, and the test coverage is strong (23 new backend tests + 5 routing tests; all 86 CrowProvider tests pass locally). One parity issue blocks approval.

Critical Issues

None.

Code Quality

🟡 listAssigned queries only --status open and silently drops in-progress tasks

CorveilTaskBackend.listAssigned fetches the open half with corveil task list --assignee @me --status open (Backends/CorveilTaskBackend.swift:131-137). Corveil's status vocabulary is open / in_progress / closed, where in_progress is a distinct status value, so --status open filters server-side to literally status == open and excludes in-progress work.

This breaks parity with the sibling backends, which both return everything not closed and treat the active/review stage as an orthogonal dimension:

  • GitHub: assignee:@me state:open type:issuestate:open = not-closed; in-progress is a project-board field (GitHubTaskBackend.swift:48).
  • Jira: assignee = currentUser() AND statusCategory != Done (JiraTaskBackend.swift:47).

The concrete consequence: setTaskStatus maps .inProgress/.inReviewin_progress (CorveilTaskBackend.swift:262-269), which is exactly what happens when a Crow session starts work on a ticket. On the next IssueTracker.refresh poll, fetchCorveilIssueslistAssigned(includeClosed: false) will no longer return that task, so the ticket you're actively working vanishes from the assigned board.

The code itself shows the intent mismatch: parseAssigned for the open call passes statusOverride: nil and maps in_progress → .inProgress (CorveilTaskBackend.swift:314), and testListAssignedSendsAtMeAndOpenStatus (CorveilTaskBackendTests.swift:716-735) feeds an in_progress task into the open response and asserts projectStatus == .inProgress — but the live --status open query will never deliver that task to the parser. The unit test passes only because FakeShellRunner returns whatever it's handed regardless of the argv filter.

Suggested fix: fetch not-closed (e.g. issue both --status open and --status in_progress, or use a "not closed" filter if corveil supports one) so in-progress tasks remain on the board, matching GitHub/Jira semantics.

Security Review

Strengths:

  • No secrets handled — auth is delegated entirely to the corveil CLI (corveil login), consistent with the Jira/GitLab pattern.
  • All CLI args are passed as a discrete argv array via ShellRunner (no shell string interpolation), so user-supplied titles/labels/URLs can't inject shell commands.
  • URL/id parsing in CorveilTaskID.parse is strict (requires a numeric suffix) and rejects unparseable input cleanly.

Concerns: None.

Minor (non-blocking)

  • 🟢 taskBackend(forURL:) detects a self-hosted host but the .corveil factory case ignores host (it threads CorveilConfig, not host), so the host-built URL fallback won't fire for URL-driven ops on self-hosted instances. This matches the existing Jira path and the fallback is rarely taken post-corveil#1363, so impact is low — just noting it.
  • 🟢 looksUnauthenticated substring-matches "unauthorized"/"not authenticated", which could theoretically misclassify an unrelated error as an auth hint. Identical to JiraTaskBackend and only affects hint text — fine as-is.

Summary Table

Color Meaning Verdict effect
Red Must fix Request changes
Yellow Should fix Request changes
Green Consider Approve allowed

Recommendation: Request Changes — driven by [0 Red, 1 Yellow, 2 Green] findings.


🐦‍⬛ Reviewed by Crow via Claude Code

Addresses PR #501 review feedback. Corveil's `--status` is an exact
match on a single status value, not "not closed" semantics — so a
`--status open` query excludes in_progress tasks. The concrete
consequence: a task we just moved to in_progress via
`setTaskStatus(.inProgress)` would vanish from the assigned board on
the next `IssueTracker.refresh` poll because `fetchCorveilIssues`
→ `listAssigned(includeClosed: false)` would no longer return it.

`listAssigned` now issues one call for `--status open` and one for
`--status in_progress`, merging the results into the open half so
the "not closed" parity matches GitHub (`state:open`) and Jira
(`statusCategory != Done`).

- Factors out the per-status query into a `listByStatus(_:)` helper.
- Replaces `testListAssignedSendsAtMeAndOpenStatus` (which fed an
  in_progress task into the "open" response — a no-op under the old
  argv filter) and updates `testListAssignedIssuesSecondCallWhenIncludeClosed`
  to expect three calls (open + in_progress + closed).
- Updates ADR 0005's TaskBackend status row to describe the fan-out.

🐦‍⬛ Generated with Claude Code, orchestrated by Crow

Co-Authored-By: Claude <[email protected]>
Crow-Session: F5DEB949-5B6F-4878-9D00-306312BBE797
@dgershman dgershman requested a review from dhilgaertner June 12, 2026 00:01

@dhilgaertner dhilgaertner left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code & Security Review

Mirrors the established JiraTaskBackend shape over the corveil CLI cleanly. Verified locally: swift test --package-path Packages/CrowProvider passes (XCTest suite + 49 swift-testing cases, including the 23 new CorveilTaskBackendTests and 6 Corveil routing tests); CrowCore builds clean. (CrowUI/root app couldn't link here due to a missing GhosttyKit.xcframework binary — an environment limitation unrelated to this PR; those touched files are mechanical and follow the existing Jira/GitLab patterns.)

Critical Issues

None.

Security Review

Strengths:

  • All corveil invocations go through ShellRunner.run(args:env:cwd:), which ProcessShellRunner executes via /usr/bin/env with an argv vector — no shell interpolation, so titles/bodies/labels/logins carrying shell metacharacters cannot inject commands.
  • No secrets handled in-process: auth lives entirely in the corveil CLI (corveil login), and the corveilHost field is explicitly URL-routing-only (documented in AppConfig.swift:9-13).
  • looksUnauthenticated surfaces a clear, non-sensitive "run corveil login" hint instead of leaking raw failure internals.

Concerns: None.

Code Quality

Solid throughout. A few optional Green / consider notes (none blocking):

  • Self-hosted URL detection is inert at runtime. additionalCorveilHosts is only ever populated in tests — AppDelegate constructs ProviderManager() with no args, so pasting a self-hosted Corveil ticket URL into the attach flow routes to .github. This exactly mirrors the existing additionalGitLabHosts gap (also never wired into the live ProviderManager), and the polling/listing path works fine since IssueTracker passes CorveilConfig(host:) directly. The PR's own test plan leaves this smoke-test item unchecked. Consistent pre-existing limitation, reasonable as a follow-up — flagging so it's a conscious choice.
  • listAssigned open-half is all-or-nothing. listByStatus("open") and listByStatus("in_progress") share one do/catch, so a transient failure on the in_progress call discards the already-fetched open results for that cycle (CorveilTaskBackend.swift:135-144). Self-heals next poll and matches the best-effort degrade contract, but preserving the open results on a partial failure would be slightly more robust.
  • Minor duplication: the host→/dashboard/tasks/{id} URL build appears in both browseURL(for:) and inline in parseAssigned (CorveilTaskBackend.swift:249-254 and 325-330) — could share one helper.

Summary Table

Color Meaning Verdict effect
Red Must fix Request changes
Yellow Should fix Request changes
Green Consider Approve allowed

Recommendation: Approve — driven by [0 Red, 0 Yellow, 3 Green] findings.


🐦‍⬛ Reviewed by Crow via Claude Code

@dgershman dgershman merged commit fb04f31 into main Jun 12, 2026
2 checks passed
@dgershman dgershman deleted the feature/crow-495-corveil-task-backend branch June 12, 2026 00:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

crow:merge Crow auto-merge on green

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Implement real CorveilTaskBackend (replace stub, follow JiraTaskBackend template) — depends on corveil#1362-1365

2 participants